Skip to content

chore: use CachePadded in a cached blockstore hot loop#6679

Merged
LesnyRumcajs merged 1 commit intomainfrom
add-cache-padded-to-atomic-pair
Mar 3, 2026
Merged

chore: use CachePadded in a cached blockstore hot loop#6679
LesnyRumcajs merged 1 commit intomainfrom
add-cache-padded-to-atomic-pair

Conversation

@LesnyRumcajs
Copy link
Copy Markdown
Member

@LesnyRumcajs LesnyRumcajs commented Mar 3, 2026 •
edited by coderabbitai Bot
Loading

Summary of changes

See https://docs.rs/crossbeam/latest/crossbeam/utils/struct.CachePadded.html

I wouldn't assume huge performance gain, but we already have crossbeam-utils in the transitive dependencies.

Changes introduced in this pull request:

  • wrap the AtomicUsize in CachePadded to avoid invalidating cache lines on every metric update.

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Outside contributions

  • I have read and agree to the CONTRIBUTING document.
  • I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

  • Refactor
    • Optimized internal cache statistics counters to improve performance in concurrent scenarios.

@LesnyRumcajs LesnyRumcajs requested a review from a team as a code owner March 3, 2026 09:50
@LesnyRumcajs LesnyRumcajs requested review from akaladarshi and hanabi1224 and removed request for a team March 3, 2026 09:50
@LesnyRumcajs LesnyRumcajs enabled auto-merge March 3, 2026 09:50
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 3, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bad9cb1 and 6c27bcc.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • Cargo.toml
  • src/db/blockstore_with_read_cache.rs

Walkthrough

Added crossbeam-utils dependency and wrapped atomic counter fields in DefaultBlockstoreReadCacheStats with cache padding to reduce false sharing. All operations on wrapped atomics are preserved; no control flow or API changes.

Changes

Cohort / File(s) Summary
Dependency Addition
Cargo.toml		 Added crossbeam-utils 0.8 dependency to support cache-padded atomic types.
Cache Padding Optimization
src/db/blockstore_with_read_cache.rs		 Wrapped hit and miss AtomicUsize fields in DefaultBlockstoreReadCacheStats with CachePadded to reduce false sharing; accessor methods unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • akaladarshi
  • hanabi1224
  • elmattic
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: adding CachePadded wrapper to atomic counters in the blockstore read cache to reduce false sharing in hot loops.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-cache-padded-to-atomic-pair

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 3, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.51%. Comparing base (7874c1d) to head (6c27bcc).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
src/db/blockstore_with_read_cache.rs 93.90% <ø> (ø)

... and 15 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7874c1d...6c27bcc. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@LesnyRumcajs LesnyRumcajs added this pull request to the merge queue Mar 3, 2026
Merged via the queue into main with commit 088ebcb Mar 3, 2026
34 checks passed
@LesnyRumcajs LesnyRumcajs deleted the add-cache-padded-to-atomic-pair branch March 3, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hanabi1224 hanabi1224 hanabi1224 approved these changes

@akaladarshi akaladarshi Awaiting requested review from akaladarshi akaladarshi is a code owner automatically assigned from ChainSafe/forest

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@LesnyRumcajs @hanabi1224